-
-
Notifications
You must be signed in to change notification settings - Fork 218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check the primary connection and assume primary if no response is found. #197
base: master
Are you sure you want to change the base?
Conversation
This guards against the case of stale shared memory that can happen when the primary application crashes or is forcefully killed. Issue itay-grudev#190 Signed-off-by: Adam Treat <[email protected]>
I get what you're trying to do here, and it's clever, but it creates a new issue. For a happy case where the primary is working correctly we end up triggering an SingleApplication/singleapplication_p.cpp Lines 267 to 286 in 494772e
I think this can be avoided if we pass an argument to We add a new enum ConnectionType : quint8 {
...
ConnectionTest = 4
}; We pass it to if ( inst->primary != false && !d->connectToPrimary( timeout, SingleApplicationPrivate::ConnectionTest )) { In if( connectionType == SingleApplicationPrivate::ConnectionTest ){
socket->close();
return true;
} None of this is tested. I leave it to you for implementation details as I can't really spend time on this project right now. But of course PR's are most welcome. |
Ah, yeah, that'll have to be fixed, but otherwise how is this approach? Does it dissuade you from the wholesale rewrite trying to eliminate the QSharedMemory? Really, with this fix the project meets our needs pretty well. |
@manyoso I don't mind accepting your PR and I'll release it as ...but I still want to do the rewrite. It will be a good improvement and I'll do my best to keep the API backwards compatible. But it has to be done, since Qt changed the implementation behind the Most of my work is client driven though. That's how this project started in the first place. I had a client who needed a more flexible Single Application solution which they described as: "The way Skype works! Click it again - it opens the existing app". I then had another client which paid for the v3 rewrite. Unfortunately I haven't had any new Qt projects to justify the v4 work and my focus has been elsewhere. So with just my free time I can support the project, but can't allocate the time it really needs. |
A Your patch will improve the status quo, but there are still problems. I need to run the That's kind of what the |
Okay, thanks for letting me know. Just as an FYI we're planning to use this in GPT4All: https://github.com/nomic-ai/gpt4all :) |
This guards against the case of stale shared memory that can happen when the primary application crashes or is forcefully killed.
Issue #190